build: remove duplicate C++ standard flags from LIEF#62682
Open
om-ghante wants to merge 2 commits intonodejs:mainfrom
Open
build: remove duplicate C++ standard flags from LIEF#62682om-ghante wants to merge 2 commits intonodejs:mainfrom
om-ghante wants to merge 2 commits intonodejs:mainfrom
Conversation
LIEF's lief.gyp explicitly sets -std=gnu++17 in cflags_cc and xcode_settings, while common.gypi already sets -std=gnu++20 project-wide. This results in both flags being passed to the compiler (-std=gnu++20 -std=gnu++17). Since the last flag wins, LIEF was silently compiling as C++17 instead of the intended project-wide C++20. Remove the explicit -std=gnu++17 flags from cflags_cc and xcode_settings.OTHER_CPLUSPLUSFLAGS, and the msvs_settings LanguageStandard override (stdcpp17), so LIEF uses the project-wide C++20 standard. LIEF is compatible with C++20 because SPDLOG_USE_STD_FORMAT is not defined, so spdlog uses its bundled fmt library rather than std::format, avoiding any C++20 conflicts. Note: a separate issue with a stray debug string in the defines list is addressed in a follow-up PR. Fixes: nodejs#62129
Collaborator
|
Review requested:
|
When building LIEF with C++20 standard, the unqualified calls to `format` and `join` in `Section.cpp` conflict with the C++20 `std::format` header provided by libstdc++ and MSVC. `std::format` attempts to evaluate these calls at compile time but fails because `fmt::join_view` is not a valid `std::formatter` type. Explicitly use `fmt::format` and `fmt::join`, and convert the joined views into `std::string` values prior to passing them into the final formatting calls to prevent the compiler from instantiating `std::format`. Fixes CI build failures on Linux ARM, macOS, and Windows. Refs: nodejs#62682
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LIEF's lief.gyp explicitly sets
-std=gnu++17incflags_ccandxcode_settings, while common.gypi already sets-std=gnu++20project-wide. This results in both flags being passed to the compiler
(
-std=gnu++20 -std=gnu++17). Since the last flag wins, LIEF wassilently compiling as C++17 instead of the intended project-wide C++20.
Additionally, upgrading LIEF to C++20 triggered compile-time conflicts in Section.cpp on modern compilers (like GCC 14 and Xcode 16.4) due to overlapping definitions between
{fmt}and C++20std::format.Changes
-std=gnu++17fromcflags_ccandxcode_settings.OTHER_CPLUSPLUSFLAGS.msvs_settingsLanguageStandard: stdcpp17override.using namespace fmt;and explicitly qualifiedfmt::formatandfmt::joininside deps/LIEF/src/PE/Section.cpp. Joined sequence views are now cleanly materialized intostd::stringobjects prior to formatting. This completely prevents the compiler from evaluating{fmt}function calls under C++20's strictstd::formatcompile-time rules, resolving the CI build crash on Linux ARM, macOS, and Windows.Why C++20 is safe for LIEF
SPDLOG_USE_STD_FORMATis not defined in the build, so spdlog uses itsbundled fmt library rather than
<format>, avoiding any C++20 conflicts once the Section.cpp syntax is properly isolated.Risk
LIEF was previously compiling as C++17 (last flag wins). With this change it
will compile as C++20. While C++20 is a superset, reviewers should confirm
no subtle deprecations or narrowing issues arise.
Note: A separate stray debug string issue in the same file is addressed
in PR #62683.
Fixes: #62129